Skip to content

Fix: Stop incrementing hit counter while client is blocked #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix: Stop incrementing hit counter while client is blocked #63

wants to merge 1 commit into from

Conversation

@JaeMyeongSon
Copy link

@JaeMyeongSon JaeMyeongSon commented Apr 23, 2025 •
edited
Loading

1. Why — Problem Statement

  • The original Lua script kept calling INCR on hitKey even while the client was already blocked.
  • Because the counter kept rising, the first request after the block TTL expired was still above limit, which triggered an immediate re-block.
  • Users therefore experienced what looked like a “permanent” ban.

2. What Changed

Step Old Logic New Logic
① Check blockKey Skipped (checked later) First: if blockKey exists → return current stats without INCR
② Increment hits Always ran INCR Runs only when not blocked
③ Unblock flow If block-TTL ≤ 0:
• DEL blockKey
• SET hitKey 1 (forced reset)		 If block-TTL ≤ 0:
• DEL blockKey (no forced reset) → hit counter expires naturally
④ Return tuple May immediately re-block Returns { totalHits, ttl, 0/1, blockTTL }; re-block loop removed

3. Behavior Before vs After (Quick Example)

Config: limit = 5, ttl = 10 s, blockDuration = 30 s

Sequence Old Response New Response
Request 1-5 200 OK 200 OK
Request 6 429 Blocked 429 Blocked
Request 7-9 (still blocked) 429 — hits keep rising 429 — hits do not rise
After 30 s Request 10 re-blocked Request 10 = 200 OK

4. Backward Compatibility

No breaking changes. The service interface and returned fields stay the same.

5. How to Test Quickly

  1. Set limit = 2, ttl = 5 s, blockDuration = 10 s.
  2. Send 3 rapid requests → the 3rd should return 429.
  3. While blocked, send 2 more requests.
    Expected: totalHits stays fixed, timeToBlockExpire counts down.
  4. Wait 10 s, send one more request.
    Expected: 200 OK, fresh hit count starts from 1.

@changeset-bot
Copy link

changeset-bot bot commented Apr 23, 2025

⚠️ No Changeset found

Latest commit: fdc153b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@JaeMyeongSon